fix(query)!: reject a non-IFC type name in ofType(), but not standard types the enum table omits - #3009

No actionable comments were generated in the recent review. 🎉

sequenceDiagram
  participant Caller
  participant IfcQuery
  participant TypeOracle
  participant IfcStore
  Caller->>IfcQuery: ofType(entityName)
  IfcQuery->>TypeOracle: validate trimmed name
  TypeOracle-->>IfcQuery: known name or Unknown mapping
  IfcQuery->>IfcStore: construct query
  TypeOracle-->>IfcQuery: reject invalid name
  IfcQuery-->>Caller: throw descriptive error

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

Viewer benchmark

✅ No threshold regressions detected.

01_Snowdon_Towers_Sample_Structural(1).ifc

Baseline recorded 2026-07-01T20:31:05.538Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.

AC20-FZK-Haus.ifc

Baseline recorded 2026-07-01T20:30:59.972Z on github-actions ubuntu-latest, viewer-benchmark-ci (headless Chrome, SwiftShader ANGLE), production build.

Refresh the baseline from a CI run: dispatch the Benchmark workflow with record_baseline, download the benchmark-baseline artifact, and commit baseline.json (see tests/benchmark/README.md).

The guard rejects seven entity names that this repo's own parser ships as real IFC entities.

packages/query/src/ifc-query.ts:118 uses IFC_ENTITY_NAMES as the oracle for "is this a real IFC entity name". That table (packages/data/src/ifc-entity-names.ts) is IFC4X3-only, 880 entries. packages/parser/src/generated/schema-registry.ts carries 776 IFC4 entities, and 15 of those are absent from IFC_ENTITY_NAMES. Eight of the 15 are still reachable because TYPE_STRING_TO_ENUM has a row for them, so the guard never runs. The other seven now throw:

• IfcDoorStyle
• IfcWindowStyle
• IfcWallElementedCase
• IfcSlabElementedCase
• IfcPresentationStyleAssignment
• IfcBuildingElement
• IfcBuildingElementType

Run against the built @ifc-lite/data with the exact predicate from this PR:

OK     IfcWall
OK     IfcChiller
THROWS IfcDoorStyle
THROWS IfcWindowStyle
THROWS IfcWallElementedCase
THROWS IfcSlabElementedCase
THROWS IfcPresentationStyleAssignment
THROWS IfcBuildingElement
THROWS IfcBuildingElementType
THROWS IfcWal
OK     Unknown

Concrete failure: IfcDoorStyle and IfcWindowStyle are how IFC2X3 files carry door and window typing, and IFC2X3 is a schema this parser reads. Today query.ofType('IfcDoorStyle') on such a file resolves to Unknown and returns those entities, which is the same "answers the query correctly in a file whose unclassified entities are of that type" case the changeset promises to preserve for IfcChiller. After this PR the call throws "IfcDoorStyle" is not an IFC entity name - check the spelling, and the name is spelled correctly.

So the changeset's "Standard IFC types that this build's enum table does not map ... are not rejected" is not true as written, and the error text tells the user to fix a spelling that is already right.

The oracle needs to cover the schemas the parser actually reads, not just IFC4X3. Widening it to the parser's schema registry (or adding the missing names to IFC_ENTITY_NAMES) would close this. The typo case this PR is aimed at still gets caught either way.

A cheap regression test: assert that every entity name in packages/parser/src/generated/schema-registry.ts passes ofType() without throwing. The current fixture picks five names by hand, and all five happen to be in IFC_ENTITY_NAMES, so it cannot see this.

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
ifc-lite-dev	Ignored	Preview	Aug 23, 2026 2:43pm
ifc-lite-viewer-embed	Ignored		Aug 23, 2026 2:43pm

Fixed, pushed as 30e19267. Your table reproduced byte-for-byte before the change; after it, every row is OK except THROWS IfcWal.

I took neither of your two options, and I think there is a better one

You offered widening to SCHEMA_REGISTRY, or adding names to IFC_ENTITY_NAMES. The predicate already exists: isKnownType() in packages/parser/src/ifc-schema.ts — the bundled IFC2X3 + IFC4 + IFC4X3 union, minus EXPRESS defined types, with the IFC4_ADD2_TC1 pin as fallback. It was built for this exact bug class in #2003, which "rejected roughly 251 perfectly valid classes".

Against your two: SCHEMA_REGISTRY is IFC4_ADD2_TC1 only, so widening to it would have added a 1.6 MB single-schema module for strictly less coverage than the oracle already available. Hand-adding names to IFC_ENTITY_NAMES is a list that drifts. Reusing isKnownType keeps one source of truth — the same predicate that guards @ifc-lite/sdk's addEntity.

No dependency problem: packages/query already value-imports from @ifc-lite/parser, so no new edge and no bundle cost.

One addition on top, and it found a case your table did not reach. isKnownType deliberately does not resolve ENTITY_NAME_ALIASES — it doubles as a name canonicalizer, and an alias maps a leaf to its supertype. A pure known-ness question does want that table, so the guard consults it too. That caught IfcElectricalDistributionPoint, an IFC2X3 name no bundled EXPRESS export carries. It threw before; it passes now.

Registry coverage, measured rather than assumed

Since your suggestion named SCHEMA_REGISTRY specifically, I checked what it actually covers: IFC4_ADD2_TC1 only. The three per-version tables union to 1159 names and contain all 776 registry entities, 0 outside. Under the final predicate SCHEMA_REGISTRY.entities rejected = 0, and the only union rows still rejected are the 6 EXPRESS defined types the parser deliberately subtracts (IfcArcIndex, IfcBinary, …) — correctly not entity names. Also confirmed 0 names in IFC_ENTITY_NAMES newly throw.

The exhaustive test, and what its failure pattern proves

Your cheap regression test, generalised: the five hand-picked names are replaced by four sweeps — SCHEMA_REGISTRY.entities plus each of IFC2X3 / IFC4 / IFC4X3.

Against the unfixed predicate, 6 tests fail, and the pattern is diagnostic rather than just red: registry, IFC2X3 and IFC4 sweeps fail while IFC4X3 passes, which isolates "IFC4X3-only oracle" exactly. Registry rejects 7 names.

Both directions pinned: NOT_IFC_ENTITY_NAMES — IfcWal, IfcWalll, Wall, a vendor name, '', IfcLengthMeasure — must still throw, and a further test asserts isKnownType(bad) === false for each, so a future change making the guard a no-op fails rather than quietly passing the sweeps.

The prose

You were right that the changeset was false. The specific claim was "The check is keyed on IFC_ENTITY_NAMES, the full IFC4X3 entity-name table" — neither full nor a schema oracle. Changeset rewritten, and I am updating the PR body to:

The check is keyed on isKnownType() (@ifc-lite/parser) — the bundled IFC2X3 + IFC4 + IFC4X3 schema union, minus EXPRESS defined types, with the IFC4_ADD2_TC1 codegen pin as fallback, plus the parser's alias table for IFC2X3 leaves the EXPRESS exports omit. It is the same predicate that guards @ifc-lite/sdk's addEntity, so there is one source of truth for "is this a real IFC class", not two. The suite asserts this exhaustively — every entity in SCHEMA_REGISTRY and in all three per-version tables — rather than by sampling.

Error message rewritten too, since "check the spelling" was wrong advice for a correctly-spelled unmapped name:

ofType(): "IfcWal" is not an entity name in any IFC schema this build reads (IFC2X3, IFC4, IFC4X3). Check the spelling; for a vendor-specific type name, pass 'Unknown' to query entities whose type could not be classified.

Semver stays major — ofType() still throws for names it previously accepted (typos, vendor names), which is a behaviour change on a published export. No public API surface moved; check-api-surface matches at 4212 exports, the oracle was already exported.

query 207 passed across 15 files (baseline 192, all +15 new); data 159, sdk 182, cli 435, mcp 272. Full 45/45 build before check-api-surface, typecheck-tests and check-changesets clean, oxlint clean.

Reviewed by reading and running. The change itself is right, and the reasoning that led to widening the oracle is the good part — catching that IFC_ENTITY_NAMES is a curated 138-entry subset, and that keying on it would reject standard IFC4 types like IfcChiller and IfcDoorStyle, is exactly the check that distinguishes a real fix from a plausible one.

Two things.

1. The body describes an implementation this PR no longer has

The body explains the fix as:

if (upper !== 'UNKNOWN' && IFC_ENTITY_NAMES[upper] === undefined) { throw … }

The head commit is "widen ofType()'s oracle to every schema the parser reads", and the code now keys on isKnownType plus resolveEntityNameAlias. So the whole IFC_ENTITY_NAMES discussion — including the "indexed-value check rather than in" rationale — describes a version that was replaced.

Worth refreshing before merge, because a reviewer reading top-down evaluates the wrong oracle, and the current one has a property the old rationale was specifically arguing about.

2. The adopted oracle accepts Object.prototype member names

Not introduced by this PR, and not yours to fix here — but it slightly weakens what this guard can promise, so it is worth knowing:

isKnownType('constructor')     -> true
isKnownType('toString')        -> true
isKnownType('valueOf')         -> true
isKnownType('hasOwnProperty')  -> true
isKnownType('__proto__')       -> true
isKnownType('isPrototypeOf')   -> true
isKnownType('NotAThing')       -> false     <- correctly rejected

Root cause is packages/parser/src/generated/schema-registry.ts:63163:

return normalized in SCHEMA_REGISTRY.entities;

in walks the prototype chain. getEntityInfoAcrossSchemas is safe — it uses a Map — so this is the one lookup that leaks.

For ofType() the consequence is mild: those six names skip the throw and fall back to the old silent-empty-result behaviour, which is the bug being fixed, for a set of strings nobody types on purpose.

Where it is not mild is the SDK authoring guard, which the code comment here cites as the reason to reuse this predicate (sdk/src/index.ts:41, sdk/src/namespaces/store.ts:71):

constructor  known=true  instantiable=true  normalizeIfcTypeName -> "Object"
toString     known=true  instantiable=true  normalizeIfcTypeName -> "toString"
__proto__    known=true  instantiable=true  normalizeIfcTypeName -> undefined

isInstantiable does not filter them either, and the normalizer turns constructor into Object and __proto__ into undefined. So an authoring call with one of those type names passes both guards and produces a garbage entity name rather than being rejected.

I am raising it here rather than filing over it, since this PR is where the oracle choice is being made and you may want a sentence in the comment acknowledging the limit. Happy to file it separately as a parser defect if you would rather keep this PR clean — it is a one-line change at the leak (Object.hasOwn(...) instead of in), but the file is generated, so the real fix is in the generator.

Both done, pushed a52497aa2. Body refreshed for point 1. Point 2 turned out to be more than either of us said.

It was not harmless

You framed it as pre-existing and not mine to fix here. End to end, against a store with one IFCWALL and one unclassified IFCCHILLER:

ofType("constructor")    -> ids [20]
ofType("toString")       -> ids [20]
ofType("__proto__")      -> ids [20]
ofType("hasOwnProperty") -> ids [20]

It passed the guard, mapped to IfcTypeEnum.Unknown, and returned the whole Unknown bucket — the silent wrong answer this PR exists to stop, reachable from any untrusted string.

It was a regression this PR introduced — but the body's reason was false even before

Verified against the branch's own history: the earlier IFC_ENTITY_NAMES implementation did reject all six. So the guard was real and the widening dropped it.

The rationale was wrong, though, and I would rather retract it than quietly reword it. The body claimed:

"An indexed-value check rather than in, so the lookup cannot reach Object.prototype."

Both control columns disprove it:

constructor:  OLD(indexed,upper)=false   OLD(in,upper)=false   indexed-no-upper=true
NotAThing:    OLD(indexed,upper)=false   OLD(in,upper)=false   indexed-no-upper=false

in with uppercasing rejects them equally; an indexed-value check without uppercasing accepts them equally. An indexed-value check reaches Object.prototype exactly as in does. What protected the old code was toUpperCase() — 'CONSTRUCTOR' is not a prototype member name. The protection was incidental, not designed, and the sentence is gone rather than rewritten.

So it is both jobs at once: restore a lost guard, and retract a rationale that was never true.

Fixed below isKnownType, not at it

Neither of the two levels I expected. isKnownType's own union lookup is a Map and was never exposed — the leak is its pin fallback isKnownEntity, which asked normalized in SCHEMA_REGISTRY.entities. Two functions along, getEntityMetadata indexed the same object literal and returned Object.prototype.toString, a Function, typed as EntityMetadata.

Both are public exports of @ifc-lite/parser (index.ts:140), so guarding inside isKnownType would have left the predicate wrong for every direct consumer, and the call-site option worse. The fix is in packages/codegen/src/typescript-generator.ts — the template that emits both — using Object.hasOwn, with the three committed generated registries updated to match byte for byte. A regeneration cannot bring it back.

BEFORE                                  AFTER
isKnownType("constructor")     -> true    -> false
isKnownType("__proto__")       -> true    -> false
isKnownType("NotAThing")       -> false   -> false

Reverting the production change with the tests kept: parser 2 failed, query 7 failed, codegen 1 failed. NotAThing passed throughout — the control behaved as a control.

The exhaustive sweeps still pass in both directions: query 214/214 includes the SCHEMA_REGISTRY sweep (>700 names) and all three per-version tables through ofType(), so the fix rejects nothing legitimate.

addEntity was never affected

StoreEditor.addEntity applies /^[Ii][Ff][Cc][A-Za-z][A-Za-z0-9_]*$/ before the normalizer (store-editor.ts:125), and no prototype member name starts with Ifc. Defense in depth held. isInstantiable did answer true for them via the same getEntityMetadata, and now answers false. sdk 182 and mutations 202 unchanged.

query 207 → 214, parser 622 → 624, codegen 117 → 118. Changeset now names @ifc-lite/parser: patch and @ifc-lite/codegen: patch alongside the existing @ifc-lite/query: major; tier unchanged, since nothing that was ever a real IFC class changed verdict.

One note on tsc --noEmit: it reports 89 errors in packages/codegen/generated/ifc4{,x3}/entities.ts, pre-existing and unrelated — a malformed EXPRESS UNIQUE IfcRepresentationMap[] in a generated stub. Those files are untouched by this branch. Zero errors in anything changed here.

Confirming CodeRabbit's Major at packages/query/src/ifc-query.ts:136. It is correct, and I traced it rather than taking the bot's word for it.

packages/data/src/types.ts:571:

export function IfcTypeEnumFromString(str: string): IfcTypeEnum {
  return TYPE_STRING_TO_ENUM.get(str.toUpperCase()) ?? IfcTypeEnum.Unknown;
}

It uppercases. It does not trim. So for ofType(' IfcWall '):

1. IfcTypeEnumFromString(' IfcWall ') looks up ' IFCWALL ', misses, returns Unknown.
2. The new guard computes trimmed = 'IfcWall', isKnownType('IfcWall') is true, so it does not throw.
3. typeEnums keeps the Unknown from step 1.

The query then runs against the Unknown bucket and returns entities that are not walls, with no error. The guard has affirmed the name is real and the resolution has ignored it.

What makes this worth fixing here rather than calling it pre-existing: the trim is new in this PR, and it was added to the acceptance side only. Before, both sides were equally strict, so a padded name was simply Unknown-in, Unknown-out. Now one side is lenient and the other is not, and the disagreement between them is exactly the silent-wrong-result window. This PR's stated purpose is to stop ofType accepting names it cannot honour, so a name it accepts and cannot honour is squarely in scope.

Seam: compute trimmed once at the top and pass it to IfcTypeEnumFromString as well, so one normalisation feeds both the lookup and the guard.

A test that pins it needs a name that is in the enum table, padded. ' IfcWall ' works; ' IfcDoorStyle ' does not, because that one legitimately resolves to Unknown on both paths and would pass with the defect present.

The two test-quality Majors on oftype-unknown-type.test.ts (the eight as any casts, and the try/catch filters that discard the original error) are also fair, and the second one matters more than it looks: a catch that reports only a rejected-name list cannot tell a wrong-name rejection from an unrelated crash, so the exhaustive check passes for a reason it never verified.

Not touching the branch, it is yours.

All three confirmed and fixed, pushed 02b25ef42. Your Major was exactly right, and the trap you flagged in the test was the part I would have walked into.

The Major

RED verbatim, seam reverted:

× a padded mapped name matches its own type, not the Unknown bucket
AssertionError: expected [ 20 ] to deeply equal [ 10 ]

Entity 20 is the unclassified IFCCHILLER, entity 10 the real wall. ofType(' IfcWall ') was answering with the Unknown bucket.

Your seam, used as given — one trim() at the top feeding both the lookup and the guard. IfcTypeEnumFromString in packages/data is untouched, so no public data-layer behaviour moves.

Your fixture warning confirmed by running. IfcDoorStyle returns Unknown padded and unpadded, while IfcWall is 10 unpadded and Unknown padded. So ' IfcDoorStyle ' is genuinely blind to the defect and ' IfcWall ' is the only shape that can see it. That is the fixture-symmetry trap we have hit twice today, and you called it before I got there.

Does trimming at the resolution site change any currently-correct input? No — no key in TYPE_STRING_TO_ENUM contains whitespace, so for a padded name the lookup missed unconditionally before, and for an unpadded name trim() is the identity. The only inputs whose meaning changes are padded names the table does map: exactly the defect.

Both directions pinned as named tests: unpadded ofType('IfcWall') → [10] unchanged; ofType(' IfcWal ') still throws and still quotes the caller's exact string including the padding; ofType(' IfcChiller ') still reaches [20], so the trim does not re-classify the Unknown-bucket path.

The blind catch — you were right that it matters more than it looks

Both exhaustive sweeps used catch { return true }. Replaced with namesRejectedByGuard(), which rethrows anything that is not the guard's own error. Proven by injecting a TypeError for one swept name:

• old form: AssertionError: expected [ 'IfcWall' ] to deeply equal [] — it reports a wrong-name rejection that never happened;
• new form: the TypeError surfaces at its own line.

So the sweep was passing for a reason it never verified, precisely as you said.

The eight casts

All eight were the same one: createMockStore returns IfcStoreBase, IfcQuery takes the parser's IfcDataStore, which adds required parse-time members (source, parseTime, deferred indices). A cast is genuinely needed, so it is now one documented queryFor(store: IfcStoreBase) helper doing as unknown as IfcDataStore — and the mock's own shape stays type-checked against IfcStoreBase rather than erased.

@ifc-lite/query 214 → 218 across 15 files. tsc --noEmit, typecheck-tests, oxlint, check-changesets all clean; check-api-surface 4212 exports unchanged, after a full 45/45 build so the answer is not off a stale dist. No production caller of ofType exists outside packages/query.

Stripped the codegen/parser half, pushed 999bc6c62. This branch now depends on #3069 landing first, and its CI will be red until then — by design. Saying that plainly rather than engineering around it.

Verified before stripping, not assumed

I did not want to remove our fix on the strength of a filename comparison, so the subsumption was checked empirically — temporarily swapping his files in, then restoring:

• our codegen test, unmodified, against his typescript-generator.ts: 22/22 passed
• our parser test, unmodified, against his generated schema-registry.ts: 18/18 passed

Nothing of ours is lost. His guards getEntityMetadata with hasOwnProperty.call and redefines isKnownEntity in terms of it, so the derived helpers our tests also asserted on — getAllAttributesForEntity, getInheritanceChainForEntity — are covered too.

isInstantiable is already pinned on his branch, so the two-line addition I offered is unnecessary: ifc-schema.prototype-guard.test.ts asserts it is false for constructor, toString, hasOwnProperty and __proto__, with both-direction controls. I confirmed the underlying defect the same way — isInstantiable('constructor') === true on main.

What remains here

.changeset/query-oftype-unknown-typo.md
packages/parser/src/ifc-schema.ts                      (doc comment only)
packages/parser/test/known-type-across-schemas.test.ts
packages/query/src/ifc-query.ts
packages/query/test/oftype-unknown-type.test.ts

The codegen files and both generated registries are byte-identical to upstream/main — git diff upstream/main -- packages/codegen packages/parser/src/generated is empty. The registries were regenerated via generate:ifc4 / generate:ifc4x3 rather than hand-edited, since they are committed artifacts.

The dependency, precisely

Against current main without #3069: 7 failing in oftype-unknown-type.test.ts (the six prototype names plus the rejected names really are unknown to the parser, not just to ofType()), and 2 failing in known-type-across-schemas.test.ts.

The cause is structural rather than incidental: ofType()'s oracle is isKnownType, whose pin fallback is isKnownEntity — the function #3069 fixes.

Those assertions stay rather than move, because they pin a different thing from his. His pins the predicate; ours pins that the defect no longer reaches ofType()'s boundary — which is where it produced a wrong answer rather than a wrong type verdict. Nothing was weakened, skipped or deleted to make the numbers look better.

Confirmed the dependency is the only thing outstanding: with his generated registry temporarily applied, parser is 624 passed / 2 skipped and query 218/218, both fully green.

Prose corrected so nothing overclaims

The changeset no longer names @ifc-lite/parser or @ifc-lite/codegen — we do not change codegen at all now, and the parser change is a comment. The paragraph that claimed the prototype fix is replaced by one stating the dependency. The ifc-schema.ts docblock and three test comments now attribute the own-property fix to #3063/#3069 and record that those cases only pass once it lands.

query 218 → 211 + 7 failing (218 again with #3069 applied); parser 624 → 622 + 2; codegen 118 → 117, dropping exactly the one test the strip removed.

This PR is not broken. It is blocked on #3069.

Node tests fails at 4m07s — a real failure, not a lane timeout — with exactly these:

FAIL test/oftype-unknown-type.test.ts > ofType() still rejects names that are not IFC entity names
  > rejects "constructor"
  > rejects "toString"
  > rejects "valueOf"
  > rejects "hasOwnProperty"
  > the rejected names really are unknown to the parser, not just to ofType()

Those are the #3063 names, and main still has the unguarded lookup:

packages/parser/src/generated/schema-registry.ts:63163
  return normalized in SCHEMA_REGISTRY.entities;

in walks the prototype chain, so isKnownType('constructor') is still true on main. The tests here are right and the fix simply has not landed yet. #3069 turns this green — and the last assertion in that list is deliberately about the parser rather than the query layer, so it cannot be satisfied any other way.

Main is green across its last five runs, so this is not ambient.

Nothing to do on this branch. Worth not re-running or bisecting it in the meantime, since it will fail identically until #3069 merges.

For the record on the ordering: the finding came out of reviewing this PR, which adopted isKnownType as its oracle. The residual I raised then — that the oracle accepts Object.prototype member names — became #3063, and #3069 fixes it at the generator rather than at the barrel, which also covers isInstantiable, normalizeIfcTypeName and getTypeId. So this PR surfaced the defect and is now waiting on its fix.

This PR is not broken. It is blocked on #3069, and the two were not linked.

Its Node tests failure is all and only the Object.prototype names:

× rejects "constructor"
× rejects "toString"
× rejects "valueOf"
× rejects "hasOwnProperty"
× the rejected names really are unknown to the parser, not just to ofType()

✓ rejects "IfcWal"      ✓ rejects "IfcWalll"   ✓ rejects "Wall"
✓ rejects "IFCPROPRIETARYVENDORTHING"          ✓ rejects "IfcLengthMeasure"   ✓ rejects ""

Every other rejection passes, which is the shape of a correct test against an unfixed dependency rather than a broken test.

origin/main:packages/parser/src/generated/schema-registry.ts:63163 still reads:

return normalized in SCHEMA_REGISTRY.entities;

in walks the prototype chain, so isKnownEntity('constructor') is true on main, and no amount of work in packages/query can make ofType() reject it. The last assertion in that list is explicitly about the parser rather than the query layer, so it cannot pass any other way.

#3069 fixes it at the generator, and I have just rebased it onto current main and re-ran the assertions this PR makes:

constructor        rejected
toString           rejected
valueOf            rejected
hasOwnProperty     rejected
IfcWal             rejected
IfcLengthMeasure   rejected
(empty)            rejected
IfcWall            ACCEPTED     <- control

So merging #3069 turns this green with no change here.

Two things on this PR that are still worth acting on independently, from the CodeRabbit review I confirmed earlier:

The Major at packages/query/src/ifc-query.ts:136 is real. IfcTypeEnumFromString uppercases but does not trim, so ofType(' IfcWall ') returns Unknown from step 1, the new guard computes trimmed and does not throw, and the query silently runs against the Unknown bucket. The trim is new in this PR and was added to the acceptance side only.

The try/catch filters in the exhaustive checks discard the original error, so they cannot tell a wrong-name rejection from an unrelated crash. That one matters more than it looks: the exhaustive check passes for a reason it never verified.

Not touching the branch. Flagging the dependency because this currently reads as a broken PR and it is a correct one waiting on a sibling.

Following up on the trim Major, because it is already fixed on the current head (999bc6c6) and I do not want it re-raised.

The defect is real. Verified against this branch's own packages/data:

"IfcWall"     raw=10        trimmed=10
" IfcWall "   raw=Unknown   trimmed=10     <- the window
"  IfcWall"   raw=Unknown   trimmed=10
" ifcwall "   raw=Unknown   trimmed=10

IfcTypeEnumFromString uppercases but does not trim, so a padded name missed TYPE_STRING_TO_ENUM and yielded Unknown, while a guard that trimmed found IfcWall known and let it through — a query silently running against the Unknown bucket and answering with entities that are not walls.

ifc-query.ts:140-141 now does const trimmed = t.trim() and feeds it to both the enum lookup and the guard, with a comment stating the asymmetry. That closes it, and trim() is the identity for every unpadded name so nothing that resolves correctly today changes meaning.

Worth stating why it was worth chasing rather than shrugging at: this is the shape where one normalisation feeds two consumers and only one of them gets it. Same as the guard that checked a product but not its operands, and the gate that normalised primaryDivisions but not subdivisions. The failure is never a crash — it is a confident answer computed from a different input than the caller supplied.

Also confirming the head has changed since the earlier review: it now reverts its own prototype-chain fix and hands that to #3069, which is the right split. The block described in my previous comment still applies until #3069 lands.

Confirmed by running: nothing is outstanding here beyond #3069. Nothing pushed.

Locally against head 999bc6c62, packages/query gives 7 failed / 29 passed, and the 7 are all and only the Object.prototype names — constructor, toString, valueOf, hasOwnProperty, __proto__, isPrototypeOf, plus the parser-level assertion. Every other rejection and the whole exhaustive sweep pass. #3069 is still open, so it is purely the dependency.

Both remaining CodeRabbit Majors are already fixed on the head, checked rather than assumed:

• the trim, as you noted at 11:10;
• the try/catch filters — namesRejectedByGuard (oftype-unknown-type.test.ts:69-80) now rethrows anything that is not the guard's own error;
• and grep -c "as any" on that file returns 0, so the eight casts are gone too, replaced by a single named queryFor widening.

One small correction: your 11:04 comment lists 5 failing assertions; the run produces 7 — the paste omits rejects "__proto__" and rejects "isPrototypeOf". It does not change your conclusion, which is right.

I am refreshing the PR body now — it still explains the fix as IFC_ENTITY_NAMES[upper] === undefined, an implementation the head no longer has. That was your finding #1 from 06:02 and I had only half-applied it.

Reviewed by Fable, under the arrangement where Fable and I now hold the review seat CodeRabbit vacated. Verified against the PR head, origin/main and the GitHub API rather than against the PR body's own claims.

Verdict: sound, correctly scoped, honestly declared breaking, and the test suite is unusually hard to fool. Three things to do before merge, none of them about the logic.

The two failing checks are base staleness of a specific kind, and a rerun will not fix it

Run 32565745096, head 999bc6c. One real failure, one derivative:

Node tests                    only failing step: turbo test --filter=!@ifc-lite/viewer
                              one package: @ifc-lite/query#test
                              one file:    test/oftype-unknown-type.test.ts
                              7 tests: rejects "constructor"/"toString"/"valueOf"/
                                       "hasOwnProperty"/"__proto__"/"isPrototypeOf"
                                       + "the rejected names really are unknown to the parser"
Build + WASM + Rust + Node    3s aggregator, prints node-tests: failure, exits 1

Those seven assert behaviour delivered by #3069, which merged at 2026-08-23 10:35Z — after this run's merge commit was computed. Not this PR's logic, and not the five new gate scripts either: every Check … gate step in the same job passed.

Remedy: refresh the base. A plain rerun replays the pinned merge commit and stays red. I have applied update-branch.

One caveat worth carrying: @ifc-lite/parser#test never appears in that job's log, because turbo aborted after the query failure. So this branch's new parser tests have never executed in CI. Their assertions match what #3069 shipped, so green is expected but not yet observed. Worth checking specifically on the refreshed run rather than assuming.

The change itself

IfcQuery.ofType() gains two behaviours, not one. The rejection is the declared breaking change. The trim is a second, quieter one: ' IfcWall ' previously resolved to Unknown and returned the unclassified bucket, and now returns walls. That is a silent result-set change for a padded-input caller, documented in the changeset but not the title.

In-repo impact is none — the only callers are the package's own walls()/doors() wrappers, and the viewer sandbox's bim.query.byType and the SDK's QueryBuilder.byType are separate descriptor paths. External exposure is the real blast radius, @ifc-lite/query is published and ofType is the README's first example. The major bump is correct and the migration (ofType('Unknown')) is stated in the changeset, the body, and the error message.

The oracle is generous and case-correct, and I tried to break it: IFC2X3-only classes, IFC4X3 infrastructure and even the IFC4X1 draft alignment entities are accepted (IfcAlignment2DHorizontal is in the bundled table at entities-ifc4.ts:794). EXPRESS defined types are correctly rejected.

One real design gap, pre-existing, and this PR now pins it

ofType('IfcProduct') and ofType('IfcElement') pass the guard — isKnownType answers known-ness including abstract supertypes — but are absent from TYPE_STRING_TO_ENUM, so they map to Unknown and silently return the unclassified bucket, excluding every mapped wall, door and slab.

That is the exact silent-wrong-answer this guard exists to prevent, reachable through the most common pattern users migrate from IfcOpenShell and web-ifc, where by_type("IfcElement") subtype-expands. The guard's oracle is "is this a real IFC name"; the harm criterion is "is the Unknown bucket a sane answer", and those diverge on essentially every abstract supertype.

Not a regression from this PR. But the new exhaustive sweep pins the acceptance, so fixing supertype semantics later means editing this test. Worth a follow-up issue, not a hold.

Minor: the package now ships two public ofTypes with different strictness — QueryBuilder.ofType at fluent-api.ts:27 still returns [] silently for a typo.

Tests

Both directions, and the reject direction's capability is not hypothetical: six of its cases are currently red in CI against the pre-#3069 oracle, which is the strongest possible demonstration that they can fail. The accept direction is exhaustive rather than sampled, asserting every name in SCHEMA_REGISTRY (>700) and each per-schema table (>400), with STANDARD_BUT_UNMAPPED asserting returned ids rather than mere absence of a throw. namesRejectedByGuard rethrows non-guard errors so an unrelated crash cannot read as "nothing rejected".

One shared-contract caveat: the sweep's DEFINED_TYPES subtraction mirrors the implementation's NON_ENTITY_NAMES_UPPER. If that ever wrongly swallowed a real entity name, guard and sweep would agree and this suite could not see it. The parser's own test file carries those assertions, so the hole is covered elsewhere, but this suite alone is not independent on that axis.

Before merge

1. Base refreshed (done) — confirm the seven now pass and that @ifc-lite/parser#test actually runs.
2. Trim the changeset paragraph "Depends on fix(codegen,parser): isKnownEntity must not accept Object.prototype members #3069 … which must land first". That is an internal ordering constraint and it ships verbatim into the published CHANGELOG and npm release notes, where it means nothing to a consumer.
3. Expect a trivial conflict in packages/parser/src/ifc-schema.ts:170-180, where this branch narrates fix(codegen,parser): isKnownEntity must not accept Object.prototype members #3069's fix in a file fix(codegen,parser): isKnownEntity must not accept Object.prototype members #3069 also touched.

/simplify pass. Not blocking — the guard is correct and the helper reuse is genuine. Four reductions, reported rather than applied since they are authoring choices in your code, not part of the rebase.

First, the thing I went looking for and did not find: isKnownType and resolveEntityNameAlias are the real existing exports (packages/parser/src/ifc-schema.ts:185 and :63), already public and already consumed by packages/sdk/src/namespaces/store.ts:71 and packages/export/src/attribute-slot-types.ts:122. No new name table. The changeset's claim of one source of truth holds.

1. packages/query/src/ifc-query.ts:145 — the first disjunct cannot change the answer

isKnownType(trimmed) || isKnownType(resolveEntityNameAlias(trimmed))

resolveEntityNameAlias returns its input for anything that is not an alias key, so the two disjuncts can only differ for the four keys in ENTITY_NAME_ALIASES. Checked all four against both tables the oracle reads:

IfcSolidStratum                 present in 0 generated tables
IfcVoidStratum                  present in 0 generated tables
IfcWaterStratum                 present in 0 generated tables
IfcElectricalDistributionPoint  present in 0 generated tables

So isKnownType(key) is false for every alias key. When the first disjunct is true the name is not an alias and the second is identically true; when it is false the second decides. isKnownType(resolveEntityNameAlias(trimmed)) alone is exactly equivalent.

The source comment at :119-126 spends a paragraph justifying the second lookup as "the difference between accepting and rejecting those names" — which is right, and is precisely the argument for the first one being unnecessary.

2. packages/query/test/oftype-unknown-type.test.ts:258 — a third copy of the parser's subtraction

The four-way union (IFC_DATA_TYPES + SCHEMA_REGISTRY.types/.enums/.selects, uppercased) now exists in three places:

packages/parser/src/ifc-schema.ts:88            the implementation
packages/parser/test/known-type-across-schemas.test.ts:77   NON_ENTITIES (pre-existing)
packages/query/test/oftype-unknown-type.test.ts:258         DEFINED_TYPES (new)

The new file's comment says it "mirrors the parser's own subtraction", which is the cost. The sweeps at :270-288 filter with the mirrored copy, so a drift between mirror and implementation weakens the sweep silently — and the test's real oracle, isKnownType, is already imported at :41 and applies the actual one.

3. One of the two new prototype-name tests duplicates the file next door

packages/parser/src/ifc-schema.prototype-guard.test.ts:34-42 (from #3063) already runs it.each([...])('isKnownType(%s) is false'), and its docblock states its job as pinning which of the package's exported guards the defect reached.

• packages/query/test/oftype-unknown-type.test.ts:292 earns its place — it pins the guard's oracle to isKnownType, which is this branch's own contract.
• packages/parser/test/known-type-across-schemas.test.ts:147 is a straight duplicate of that neighbour, in a file whose stated topic is Pinned IFC4 registry silently skips IFC2X3/IFC4X3 classes in validate and MCP tools #2003 cross-schema known-ness. Its companion at :175 (getEntityMetadata returns undefined) is new coverage, but its natural home is ifc-schema.prototype-guard.test.ts, which already covers the sibling getTypeId for the same reason.

4. Small, same file

• GUARD_MESSAGE (:46) is defined once then bypassed: the raw regex is respelled at :153, :159, :165, :304; only :203 uses the constant.
• :150 and :156 are both already covered by the parameterized loop at :301-306 — both names are in NOT_IFC_ENTITY_NAMES and the assertion is identical.
• storeWithUnclassified:141 calls .trim() on the store's entity type; no call site passes a padded name.

Rebase status

The Depends on #3069 ... which must land first paragraph is removed from the changeset on the rebased branch — it would have shipped verbatim into @ifc-lite/query's published CHANGELOG, where a consumer cannot act on an internal merge-ordering constraint, and it is stale now that #3069 is on main as f4497765c. Consumer-facing content is unchanged: what throws, what still resolves, why the major bump, and the error text.

I could not run the suites in that worktree — no node_modules — so finding 1 is established by reading the three data sources directly rather than by executing the predicate. Saying so rather than implying I ran it.